Skip to content

gh-95005: Replace PyAccu with PyUnicodeWriter#95006

Merged
corona10 merged 4 commits into
python:mainfrom
aivarsk:remove_pyaccu
Jul 27, 2022
Merged

gh-95005: Replace PyAccu with PyUnicodeWriter#95006
corona10 merged 4 commits into
python:mainfrom
aivarsk:remove_pyaccu

Conversation

@aivarsk

@aivarsk aivarsk commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

PyAccu was still being used in only two places: the JSON encoder and StringIO.
Replace PyAccu with PyUnicodeWriter and remove PyAccu completely.

@aivarsk aivarsk requested a review from a team as a code owner July 19, 2022 12:22
@ghost

ghost commented Jul 19, 2022

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@aivarsk aivarsk marked this pull request as draft July 19, 2022 12:25
PyAccu was still being used in only two places: the JSON encoder and StringIO.
Replace PyAccu with PyUnicodeWriter and remove PyAccu completely.
@aivarsk aivarsk marked this pull request as ready for review July 19, 2022 13:28

@MaxwellDupre MaxwellDupre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only 3 tests failed from test suite. Code is good.

PyAccu was still being used in only two places: the JSON encoder and StringIO.
Replace PyAccu with PyUnicodeWriter and remove PyAccu completely.
@corona10 corona10 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 25, 2022
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @corona10 for commit a9b95a5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 25, 2022
@corona10 corona10 self-assigned this Jul 25, 2022
@corona10

Copy link
Copy Markdown
Member

@MaxwellDupre

only 3 tests failed from test suite. Code is good.

Which test?

@corona10 corona10 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 26, 2022
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @corona10 for commit a9b95a5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 26, 2022
@aivarsk

aivarsk commented Jul 26, 2022

Copy link
Copy Markdown
Contributor Author

buildbot/AMD64 Debian seems to have a broken compiler or hardware error. The compiler crashed the last time and now it gets SIGBUS while compiling classobject.c which was not modified by the patch.

@MaxwellDupre

Copy link
Copy Markdown
Contributor

I ran
./python -m test -j0 -W -uall
because of the code coverage and out of >400 tests 3 failed which IMHO was insignificant also those fails did not seem to be related to the fix. Hence, I didn't record them and thought not worth mentioning.

@corona10 corona10 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 26, 2022
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @corona10 for commit a9b95a5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 26, 2022
@corona10

Copy link
Copy Markdown
Member

buildbot/AMD64 Debian seems to have a broken compiler or hardware error. The compiler crashed the last time and now it gets SIGBUS while compiling classobject.c which was not modified by the patch.

I sent a mail to the machine owner, seems unrelated to this PR.

@corona10 corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Overall looks good, I just left nit comment.

Comment thread Modules/_json.c Outdated
@corona10 corona10 merged commit 8c88e36 into python:main Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants